Agregar markup_doc y fixtures DOCX by eduranm · Pull Request #58 · scieloorg/markapi

eduranm · 2026-04-20T13:49:35Z

O que esse PR faz?

Agrega la base del módulo markup_doc e incorpora fixtures DOCX para pruebas

Incluye:

registro de markup_doc;
incorporación del directorio markup_doc/;
incorporación del directorio fixtures/;

Onde a revisão poderia começar?

Por commits

Como este poderia ser testado manualmente?

Levantar el entorno;
Verificar que markup_doc esté registrada;
Probar el flujo base con alguno de los archivos en fixtures/.

Algum cenário de contexto que queira dar?

Deja preparada la base para continuar con las siguientes funcionalidades de markup_doc.

Screenshots

N/A

Quais são tickets relevantes?

#57

Referências

N/A

Copilot

Pull request overview

This PR introduces the initial markup_doc Django app to support a base DOCX markup flow in MarkAPI, including Wagtail admin registration and a sample DOCX fixture for manual testing.

Changes:

Added markup_doc app with initial models, Wagtail hooks/viewsets, and DB migrations.
Added external catalog sync helpers and a Celery task for journal synchronization.
Added a DOCX fixture under fixtures/ and registered markup_doc in project settings.

Reviewed changes

Copilot reviewed 11 out of 17 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`config/settings/base.py`	Registers `markup_doc` in `INSTALLED_APPS`.
`markup_doc/apps.py`	Adds the Django `AppConfig` for the new app.
`markup_doc/models.py`	Introduces core models/proxies and Wagtail edit handlers for DOCX + markup data.
`markup_doc/wagtail_hooks.py`	Registers Wagtail snippet viewsets/group and adds preconditions around collection/journal availability.
`markup_doc/sync_api.py`	Implements external API sync for collections/journals.
`markup_doc/tasks.py`	Adds a Celery task wrapper for journal sync.
`markup_doc/migrations/0001_initial.py`	Creates initial schema for the new app.
`markup_doc/migrations/0002_alter_articledocx_estatus_and_more.py`	Aligns `estatus` field choices/defaults.
`markup_doc/forms.py`	Placeholder forms module (currently only an import).
`markup_doc/choices.py`	Adds label/ordering constants used by Wagtail blocks.
`markup_doc/tests.py`	Placeholder test module.
`fixtures/e14790.docx`	Adds a sample DOCX file for manual testing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T14:17:33Z

+    def update(cls, title, estatus):
+        try:
+            obj = cls.get(title=title)
+        except (cls.DoesNotExist, ValueError):
+            pass
+
+        obj.estatus = estatus
+        obj.save()
+        return obj


ArticleDocx.update() swallows DoesNotExist/ValueError and then uses obj anyway, which will raise UnboundLocalError when the record doesn't exist. Decide on the intended behavior (re-raise, return None, or create a new object) and implement it explicitly.

Copilot · 2026-04-24T14:17:33Z

+    def update(cls, title, estatus):
+        try:
+            obj = cls.get(title=title)
+        except (cls.DoesNotExist, ValueError):
+            pass
+
+        obj.estatus = estatus
+        obj.save()
+        return obj


ArticleDocxMarkup.update() has the same issue as ArticleDocx.update(): if the object doesn't exist, the exception is swallowed and obj is still referenced, causing UnboundLocalError. Implement explicit behavior for the not-found case.

Copilot · 2026-04-24T14:17:33Z

+        return str(self)
+
+    def __str__(self):
+        return self.title


JournalModel.__str__() returns self.title, but title is nullable; returning None from __str__ raises TypeError. Ensure __str__ always returns a string (e.g., fallback to an empty string or another identifier).

Suggested change

return self.title

return self.title or ""

Copilot · 2026-04-24T14:17:34Z

+import os
+import sys
+import requests
+
+from django.db import models
+from django.utils.translation import gettext_lazy as _
+from django import forms
+from django.utils.html import format_html
+from django.urls import reverse
+
+from modelcluster.fields import ParentalKey
+from modelcluster.models import ClusterableModel
+from wagtail.admin.panels import FieldPanel, InlinePanel, ObjectList, TabbedInterface
+from wagtailautocomplete.edit_handlers import AutocompletePanel
+from wagtail.documents.models import Document
+
+from core.forms import CoreAdminModelForm
+from core.choices import LANGUAGE
+from core.models import (
+    CommonControlField,
+    Language,
+    TextWithLang
+)
+from wagtail.fields import StreamField
+from wagtail.blocks import StructBlock, TextBlock, CharBlock, ChoiceBlock, ListBlock, StreamBlock


This module currently has several unused imports (e.g., os, sys, requests, ParentalKey, InlinePanel, Document, Language, TextWithLang, CharBlock, ListBlock). With flake8 enabled, these will fail CI (F401). Remove unused imports or start using them in the implementation.

Suggested change

import os

import sys

import requests

from django.db import models

from django.utils.translation import gettext_lazy as _

from django import forms

from django.utils.html import format_html

from django.urls import reverse

from modelcluster.fields import ParentalKey

from modelcluster.models import ClusterableModel

from wagtail.admin.panels import FieldPanel, InlinePanel, ObjectList, TabbedInterface

from wagtailautocomplete.edit_handlers import AutocompletePanel

from wagtail.documents.models import Document

from core.forms import CoreAdminModelForm

from core.choices import LANGUAGE

from core.models import (

CommonControlField,

Language,

TextWithLang

)

from wagtail.fields import StreamField

from wagtail.blocks import StructBlock, TextBlock, CharBlock, ChoiceBlock, ListBlock, StreamBlock

from django.db import models

from django.utils.translation import gettext_lazy as _

from django import forms

from django.utils.html import format_html

from django.urls import reverse

from modelcluster.models import ClusterableModel

from wagtail.admin.panels import FieldPanel, ObjectList, TabbedInterface

from wagtailautocomplete.edit_handlers import AutocompletePanel

from core.forms import CoreAdminModelForm

from core.choices import LANGUAGE

from core.models import CommonControlField

from wagtail.fields import StreamField

from wagtail.blocks import StructBlock, TextBlock, ChoiceBlock, StreamBlock

Copilot · 2026-04-24T14:17:34Z

@@ -0,0 +1 @@
+from wagtail.admin.forms.models import WagtailAdminModelForm


markup_doc/forms.py only imports WagtailAdminModelForm but doesn't use or export it, which will trigger flake8 unused-import failures. Either remove this file/import or define the intended form subclass here and use it from the models/viewsets.

Suggested change

from wagtail.admin.forms.models import WagtailAdminModelForm

"""Form definitions for the markup_doc app."""

Copilot · 2026-04-24T14:17:35Z

+    def create(cls, title, doi):
+        obj = cls()
+        obj.title = title
+        obj.doi = doi


ArticleDocxMarkup.create() assigns obj.doi, but ArticleDocxMarkup doesn't define a doi field. This will raise AttributeError at runtime. Either add the missing field or remove/update the factory method signature to match the actual model fields.

Suggested change

def create(cls, title, doi):

obj = cls()

obj.title = title

obj.doi = doi

def create(cls, title):

obj = cls()

obj.title = title

Copilot · 2026-04-24T14:17:36Z

+    ]
+
+    def __str__(self):
+        return f"{self.collection.acron.upper()} - {self.collection.acron}"


CollectionModel.__str__() assumes self.collection is always set, but the FK is nullable/blank. If a row exists without a selected collection, this will raise AttributeError when rendered in admin. Guard for None (and consider displaying the collection name instead of repeating the acronym twice).

Suggested change

return f"{self.collection.acron.upper()} - {self.collection.acron}"

if self.collection is None:

return "No collection selected"

return str(self.collection)

Copilot · 2026-04-24T14:17:36Z

+class ReadOnlyFileWidget(forms.Widget):
+    def render(self, name, value, attrs=None, renderer=None):
+        if value:
+            # Muestra el archivo como un enlace de descarga
+            #return format_html('<a href="{}" target="_blank" download>{}</a>', value.url, value.name.split('/')[-1])
+            instance = value.instance
+            url = reverse('generate_xml', args=[instance.pk])
+            return format_html('<a href="{}" target="_blank" download>Download XML</a>', url)


ReadOnlyFileWidget calls reverse('generate_xml', ...), but there is no URL pattern named generate_xml in the repository (searching the codebase only finds this reference). This will raise NoReverseMatch when the admin form renders. Add the missing URL/view or change this to point at an existing route (or fall back to value.url).

Copilot · 2026-04-24T14:17:36Z

+from django.http import HttpResponseRedirect
+from django.utils.translation import gettext_lazy as _
+from django.contrib import messages
+from django.template.response import TemplateResponse
+from wagtail_modeladmin.options import ModelAdmin
+
+from wagtail.snippets.views.snippets import (
+    CreateView,
+    EditView,
+    SnippetViewSet,
+    SnippetViewSetGroup
+)
+
+from markup_doc.models import ( 
+    ArticleDocx,
+    ArticleDocxMarkup,
+    UploadDocx,
+    MarkupXML,
+    CollectionModel,
+    JournalModel,
+    ProcessStatus
+)
+
+from config.menu import get_menu_order
+from markup_doc.tasks import task_sync_journals_from_api
+from django.urls import path, reverse
+from django.utils.html import format_html
+from wagtail.admin import messages
+from wagtail.admin.views import generic
+
+from django.shortcuts import redirect, get_object_or_404
+from django.views import View
+


The import section has multiple unused / duplicated imports (e.g., importing messages from both django.contrib and wagtail.admin, plus several unused Wagtail/Django utilities). This will trigger flake8 F401 and the messages name is currently shadowed, which is error-prone. Remove unused imports and keep a single messages import (or alias one of them) to avoid shadowing.

Copilot · 2026-04-24T14:17:37Z

+class JournalModelCreateView(CreateView):
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        task_sync_journals_from_api


JournalModelCreateView.get_context_data() references task_sync_journals_from_api but never calls it, so no sync will actually run when this view is hit. Call the task (e.g., .delay()) or remove this method if it isn't needed.

Suggested change

task_sync_journals_from_api

task_sync_journals_from_api.delay()

eduranm added 3 commits April 16, 2026 13:52

Agregar la app markup_doc para gestión y marcación de documentos DOCX

2614d27

Agregar archivos DOCX de ejemplo para pruebas del flujo de marcación

e5a6783

Registrar markup_doc

3ddf5ca

robertatakenaka requested a review from Copilot April 24, 2026 14:12

Copilot started reviewing on behalf of robertatakenaka April 24, 2026 14:13 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Agregar markup_doc y fixtures DOCX#58

Agregar markup_doc y fixtures DOCX#58
eduranm wants to merge 3 commits intoscieloorg:mainfrom
eduranm:issue-02

eduranm commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1 @@
		from wagtail.admin.forms.models import WagtailAdminModelForm

	from wagtail.admin.forms.models import WagtailAdminModelForm
	"""Form definitions for the markup_doc app."""

-        return f"{self.collection.acron.upper()} - {self.collection.acron}"
+        if self.collection is None:
+            return "No collection selected"
+        return str(self.collection)

	task_sync_journals_from_api
	task_sync_journals_from_api.delay()

Conversation

eduranm commented Apr 20, 2026

O que esse PR faz?

Onde a revisão poderia começar?

Como este poderia ser testado manualmente?

Algum cenário de contexto que queira dar?

Screenshots

Quais são tickets relevantes?

Referências

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants